Add pairwise #627

nalimilan · 2021-01-01T16:22:15Z

This generic method takes iterators of vectors and supports skipping missing values.
It is a more general version of pairwise in Distances.jl.
Since methods are compatible, both packages can override a common empty function defined in another API package.

See nalimilan/FreqTables.jl#54. Cc: @dkarrasch @bkamins @quinnj

This generic method takes iterators of vectors and supports skipping missing values. It is a more general version of `pairwise` in Distances.jl. Since methods are compatible, both packages can override a common empty function defined in another API package.

src/pairwise.jl

dkarrasch

Looks pretty good, only two comments about indexing/iterating x and y and one-based indexing into res.

If I haven't missed anything, then these methods here are more generic than the generic methods in Distances.jl. I believe one could map those to the _pairwise! "kernels" here.

EDIT: Aha, I guess one difference could be that you're assuming here that you apply pairwise to iterators of (indexable) vectors, whereas pairwise in Distances.jl applies to iterators of iterators.

src/pairwise.jl

nalimilan · 2021-01-10T18:34:01Z

I've pushed commits which should address all comments. I've also made two changes:

Add pairwise! since it is standard practice to have in-place variants
When skipmissing=:none, allow entries to be any objects, and not just vectors. This is more consistent with Distances.jl as @dkarrasch noted.

bkamins · 2021-01-10T23:27:50Z

Nice. Just to go back for a second to the issue with missing. Let us confirm that we really want:

julia> x = [missing, missing, missing]
3-element Array{Missing,1}:
 missing
 missing
 missing

julia> pairwise(cor, Ref(x), Ref(x))
1×1 Array{Missing,2}:
 missing

as this is the first case when the docstring is not correct (but I think it is OK as you have no way to infer the type of "one").

The second case is:

julia> x = Union{Int, Missing}[missing, missing, missing]
3-element Array{Union{Missing, Int64},1}:
 missing
 missing
 missing

julia> pairwise(cor, Ref(x), Ref(x), skipmissing=:pairwise)
ERROR: ArgumentError: correlation only defined for non-empty vectors

(we error although we promise to return 1, but again - maybe it is OK to leave this corner case as is)

nalimilan · 2021-01-11T09:14:29Z

Good catch. Yes I'm not sure we can do much about it. Technically we return one(missing), so the docstring isn't totally wrong in the first case. :-p I'm not sure it's worth mentioning as it sounds difficult to explain without making the sentence very complex, but at least I've added tests so that we don't change the behavior without noticing. This made me discover that in some cases we threw a MethodError because the destination was Matrix{Union{}} instead of letting cor throw the expected ArgumentError.

I've also found a simple way to use promote_type rather than promote_typejoin to choose the element type, while still having the function inferable. I think this makes more sense as it gives a concrete eltype (Float64 in general) if you have columns of different types (which is common for tables), and it justifies using pairwise rather than just map(f, Iterators.product(x, y)).

bkamins · 2021-01-11T09:32:17Z

Looks good in general (but CI fails on Julia 1.0, so probably a separate code path for it is required).

dkarrasch

Indeed, looks good.

src/pairwise.jl

nalimilan · 2021-03-07T17:42:01Z

I've created a StatsAPI package, with a PR to define pairwisethere: JuliaStats/StatsAPI.jl#1

pdeffebach · 2021-03-14T22:20:02Z

Could we implement a special case for pairwise(cor, x::Vector{Number}, y::Vector{Number}) to allow scalar returns?

Or would that be a different function entirely.

nalimilan · 2021-04-04T21:25:03Z

Could we implement a special case for pairwise(cor, x::Vector{Number}, y::Vector{Number}) to allow scalar returns?

Or would that be a different function entirely.

That would have to be another function (more akin to skipmissings), as this PR already gives a meaning to this signature. Vectors of numbers are treated like any other collection:

julia> pairwise(tuple, [1, 2], [3, 4])
2×2 Matrix{Tuple{Int64, Int64}}:
 (1, 3)  (1, 4)
 (2, 3)  (2, 4)

I've added a commit to import pairwise and pairwise! from StatsAPI. With JuliaStats/Distances.jl#213, StatsBase and Distances will cohabit nicely without method ambiguities. I'll merge in a few days barring objections.

bkamins · 2021-04-05T11:22:07Z

OK. Looks good.

wildart · 2021-04-28T22:22:07Z

When is this expected to be merged? I'd like to use new APi in JuliaStats/MultivariateStats.jl#148.

nalimilan · 2021-05-02T14:41:28Z

JuliaRegistries/General#35827

nalimilan requested review from quinnj and dkarrasch January 1, 2021 16:22

nalimilan mentioned this pull request Jan 1, 2021

Add pairwise nalimilan/FreqTables.jl#54

Draft

nalimilan force-pushed the nl/pairwise branch from c94be63 to 2170bfa Compare January 1, 2021 16:35

Add pairwise

9c8b3a6

This generic method takes iterators of vectors and supports skipping missing values. It is a more general version of `pairwise` in Distances.jl. Since methods are compatible, both packages can override a common empty function defined in another API package.

nalimilan force-pushed the nl/pairwise branch from 2170bfa to 9c8b3a6 Compare January 1, 2021 16:35

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Show resolved Hide resolved

bkamins reviewed Jan 1, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

dkarrasch reviewed Jan 2, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

src/pairwise.jl Outdated Show resolved Hide resolved

nalimilan mentioned this pull request Jan 6, 2021

Define SemiMetric and Metric to be traits rather than types JuliaStats/Distances.jl#199

Open

nalimilan added 12 commits January 9, 2021 15:21

Do not propagate missings on diagonal for cor

a730ba4

Clarify "diagonal cells"

2b43466

Improve wording

da2e97c

Handle empty inputs

535a094

Add pairwise!, check that indices start at 1

a7f647e

Use @inbounds

4f8f0c6

Allow non-vector entries when skipmissing=:none

2d03726

Rename res to dest

b62b95a

Add examples

b3ae81e

More tests for pairwise!

c2c2798

More generic code

bb04b5c

Fix tests on Julia < 1.6

baab8d2

nalimilan added 3 commits January 11, 2021 09:41

Add missing using

2378f26

Improve handling of missings

bd0af8c

Use promote_type rather than promote_typejoin

88912da

Fix Julia 1.0

e52a9c0

bkamins approved these changes Jan 11, 2021

View reviewed changes

Avoid anonymous function

6bbc25b

dkarrasch reviewed Jan 11, 2021

View reviewed changes

src/pairwise.jl Outdated Show resolved Hide resolved

Fix vector check

42a940a

This was referenced Jan 17, 2021

adding Union complexF type in cor JuliaStats/Statistics.jl#65

Closed

Speeding up Kendall Correlation corkendall #634

Closed

nalimilan mentioned this pull request Feb 8, 2021

Rewrote corkendall (issue 634) #647

Merged

nalimilan mentioned this pull request Mar 7, 2021

Add pairwise JuliaStats/StatsAPI.jl#1

Merged

wildart mentioned this pull request Mar 26, 2021

fixed pairwise for non-symmetric kernels JuliaStats/MultivariateStats.jl#148

Merged

nalimilan added 2 commits April 4, 2021 23:12

Use StatsAPI

d10e3b4

Add pairwise! to docs

dc2ab9c

Merge branch 'master' into nl/pairwise

2c56df9

nalimilan merged commit d18762c into master May 2, 2021

nalimilan deleted the nl/pairwise branch May 2, 2021 14:38

nalimilan mentioned this pull request Jul 25, 2021

Added Minimal Support for Reliability Scores (aka Crombach's alpha) #701

Merged

PGS62 mentioned this pull request Jan 9, 2023

Add a skipmissing argument to corkendall #683

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pairwise #627

Add pairwise #627

nalimilan commented Jan 1, 2021

dkarrasch left a comment •

edited

Loading

nalimilan commented Jan 10, 2021

bkamins commented Jan 10, 2021 •

edited

Loading

nalimilan commented Jan 11, 2021

bkamins commented Jan 11, 2021

dkarrasch left a comment

nalimilan commented Mar 7, 2021

pdeffebach commented Mar 14, 2021

nalimilan commented Apr 4, 2021

bkamins commented Apr 5, 2021

wildart commented Apr 28, 2021

nalimilan commented May 2, 2021

Add pairwise #627

Add pairwise #627

Conversation

nalimilan commented Jan 1, 2021

dkarrasch left a comment • edited Loading

Choose a reason for hiding this comment

nalimilan commented Jan 10, 2021

bkamins commented Jan 10, 2021 • edited Loading

nalimilan commented Jan 11, 2021

bkamins commented Jan 11, 2021

dkarrasch left a comment

Choose a reason for hiding this comment

nalimilan commented Mar 7, 2021

pdeffebach commented Mar 14, 2021

nalimilan commented Apr 4, 2021

bkamins commented Apr 5, 2021

wildart commented Apr 28, 2021

nalimilan commented May 2, 2021

dkarrasch left a comment •

edited

Loading

bkamins commented Jan 10, 2021 •

edited

Loading